OK... All ready. any thoughts on when is the next release I could expect this could be merged into if it's all good for you guys?

Fixes #2275

Thanks for submitting a contribution! I can respect that you're trying to reduce the scope of the problem and target a more specific surface area (specifically, errors of a particular type thrown within the context), but I don't think we can neglect the larger discussion around the relationship of HTTP status codes and GraphQL errors.

Of course, that discussion is quickly complicated since GraphQL can return partial successes/failures within the same response and a 401 status code might only reflect a certain part of the failure. I don't believe anything I'm saying here will come as a surprise, since that's been the direction of the conversation in #1709.

I can see how it's tempting to make a 1:1 pairing between a GraphQL AuthenticationError and a 401 HTTP status code, but I'm afraid that associating HTTP status codes with GraphQL errors is not a one-size-fits-all (or even one-size-fits-many) solution to tackle, since responses in GraphQL can be a mix of errors and non-errors and the only HTTP status code which seems to come close to ticking that box is the rarely-seen 207 Multi-Status.

Concretely though, trying to narrow the scope to errors sourced from context creation by special-casing them and treating them differently than the errors within resolver execution (which this PR does) would seem to make it more difficult to educate implementors about the difference since the behavior would vary depending on the error's source. Additionally, we need to keep in mind that GraphQL does not only run over HTTP; Apollo Server supports using WebSockets and WebSockets subscribe to a completely different notion of status codes.

This would also be a major breaking change since there's no certainty that any client which is currently consuming from the current implementation is prepared for receiving a different status code; entire implementations are likely hinging on the fact that if there's an AuthenticationError, it will be represented in the GraphQL errors and not reflected in the status code (which would expect 200).

I want to make it clear that I think it's important that we provide those who need to support this for legacy reasons a way to do it, but I don't think the current implementation in this PR is correct. I think I have an idea on how we can provide that functionality (and I think that the proper API will also help resolve #631), but we should continue to think about that in the original issue and close this PR. I realize you opened this PR to try to fix it, but I don't think we had enough buy-in to adopt this approach.

Ultimately, I think we should make sure that anyone building GraphQL applications today makes sure that their client implementations are built around a multiple-response/partial-error paradigm which doesn't constrain them by singular HTTP status codes, but allows those that want different behavior to opt-into it.

Hi @abernix.

Thanks for getting back to me on this one. I get your point, I'll look at solving my issue in a different way then. The point of HTTP not being the only protocol drove it home for me. A part of me still thinks that in that case the response was not partial, the whole thing was denied, but well, I can see your PoV.
I'll focus on making the client manage the response differently. Hey, I hope that the conversation at least helps other that might have followed my same line of thought.

It's very unfortunate this PR did not go through. This behavior is incredibly problematic. Not returning 401 and not being able to reasonably patch it is simply broken from an http point of view. Partials I agree and am all for general but that does not make sense when you are rejecting the request wholistically because they aren't authenticated. Ie.. from lookup in context. Apollo Server plugin infrastructure doesn't hook in to that point as it is resolved before requestDidStart and trying to wrap it yourself atleast in the apollo-server-lambda context means trying to parse already formulated strings. This is broken.

Could someone tell how to handle a simple case when I'm checking Authorization token in request headers and if it's missing I want just to return AuthenticationError to the client? At the moment I'm getting { "error": "Response not successful: Received status code 400" } but expecting just a default 401 error.
P.S. I'm sorry if this question is nooby but it's my first project with Apollo ever :)

I worked around this issue by creating an ExpressJS middleware to promote authentication errors masquerading as user-input errors to have a 401 status:

import { Request, Response, NextFunction } from 'express'
import { GraphQLResponse } from 'apollo-server-types'

const contextAuthError = (_req: Request, res: Response, next: NextFunction) => {
  const origSend = res.send

  res.send = content => {
    if (res.statusCode === 400) {
      const errInfo: GraphQLResponse = JSON.parse(content)

      if (errInfo.errors[0].extensions.code === 'UNAUTHENTICATED') {
        res.statusCode = 401
      }
    }

    return origSend.call(res, content)
  }

  next()
}

export default contextAuthError

where this is applied immediately preceding your server middleware, like so:

app.use('/api', contextAuthError)
server.applyMiddleware({ app, path: '/api' })

I looked at a few public GraphQL implementations I could find -- Github, Yelp, and Shopify -- and they all returned a 401 when I try to query them without auth. I find it odd that Apollo Server makes this so difficult to do. I tried a couple things, such as using plugins, and returning a 401 in middleware (but this affects the /graphql UI too), without success, until I found @dchambers's post -- thank you for posting that workaround!